Auto-uninstall auto-installed modules #2753

HebaruSan · 2019-05-06T03:43:42Z

Motivation

Installing a module pulls in its dependencies automatically. If the user later uninstalls that module, they may wish to remove the dependencies as well, but there's no easy way to identify them.

Debian's APT package management system has a concept of "automatically installed packages," which handles this situation nicely:

To install one package, it is often necessary to install several others (to fulfill its dependencies). For instance, if you wish to install the clanbomber package, you must also install the package libclanlib2. If you remove clanbomber again, you probably no longer need the libclanlib2 package; aptitude will attempt to detect this and automatically remove the libclanlib2 package.

It works like this: when you install a package, aptitude will automatically install any other packages on which it depends. These packages are marked as having been “automatically installed”; aptitude will monitor them and remove them when they are no longer depended upon by any manually installed package [10] . They will appear in the preview as “packages being removed because they are no longer used.”

As with any automatic process, there is a potential for things to go haywire. For instance, even if a package was automatically installed to start with, it might turn out to be useful in its own right. You can cancel the “automatic” flag at any time by pressing m; if the package is already being removed, you can use Package → Install (+) to cancel the removal and clear the “automatic” flag.

https://www.debian.org/doc/manuals/aptitude/ch02s02s06.en.html

Changes

Now CKAN supports a concept of automatically installed modules.

Fixes KSP-CKAN/NetKAN#6825 (because the motivation behind that issue was cleaning up dependencies that are no longer needed).

GUI

When you install a mod:

... a new checkbox column appears called "Auto-installed":

This will be checked for mods that were installed due to dependencies, and unchecked for mods the user chose to install. Recommendations and suggestions are considered as user-chosen. Any modules installed before the user upgraded to this version of CKAN will be treated as user-chosen.

When you remove a mod, we check for any auto-installed modules that will no longer be needed, and automatically add them to the change set:

The user can toggle the checkboxes to control whether a module should be considered auto-installed:

If a module was previously considered auto-installed, unchecking the box will "anchor" it so that it can no longer be auto-removed. If the module was previously not considered auto-installed, checking the box will allow the module to be auto-removed; if it currently has no depending mods, it will be added to the changeset for removal immediately:

You can hide the new column, since it may not be useful to some users:

To make this work, a new property InstalledModule.auto_installed is added that will be saved to registry.json. I don't think this requires an update of the schema or the spec, because InstalledModule is a private object that CKAN creates and consumes internally.

To make the checkbox column work, we now only reference columns by name instead of by index.

CmdLine

CmdLine is updated to print "+" as the symbol for auto installed:

> _build\ckan list

KSP found at C:/Program Files (x86)/Steam/steamapps/common/Kerbal Space Program

KSP Version: 1.7.1

Installed Modules:

+ ContractConfigurator 1.27.1
- CustomBarnKit 1.1.19.0
- FinalFrontier 1.5.3-3465
- GPP 1:1.6.3.1
+ GPPTextures 4.2.1
- KerbalKonstructs 1.4.5.64
+ Kopernicus 2:release-1.7.0-1
A MakingHistory-DLC 1.7.1 (unmanaged)
+ ModularFlightIntegrator 1.2.6.0
+ ModuleManager 4.0.2
- Strategia 1.7.3
- WaypointManager 2.7.5

Legend: -: Up to date. +:Auto-installed. X: Incompatible. ^: Upgradable. >: Replaceable
        A: Autodetected. ?: Unknown. *: Broken.

You can mark modules as auto-installed via CmdLine:

> ckan mark auto gpp astrogator
Marking GPP as auto-installed...
Astrogator is not installed.
Changes made!

Or unmark them:

> ckan mark user gpp astrogator
Marking GPP as user-selected...
Astrogator is not installed.
Changes made!

ConsoleUI

ConsoleUI is updated along the same lines:

Auto-installed mods are represented with a floating 'a' character with an underline, which codepage 437 charmingly calls the "feminine ordinal" symbol
The F8 key toggles auto install
The help screen explains what's up:

DasSkelett

If I remove a mod (6CSL) with auto-installed dependencies (MM), the reason for action for the auto-removed mod is wrong:

If you clear the changeset on the changeset tab (doesn't matter if there's an "auto-remove" in the changeset or not), ckan crashes:

System.NullReferenceException: Object reference not set to an instance of an object
at System.Windows.Forms.DataGridViewCellCollection.get_Item (System.String columnName) [0x0002e] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at (wrapper remoting-invoke-with-check) System.Windows.Forms.DataGridViewCellCollection.get_Item(string)
at CKAN.GUIMod.SetUpgradeChecked (System.Windows.Forms.DataGridViewRow row, System.Nullable`1[T] set_value_to) [0x00007] in <c787e8ddab794c269a4e41cd1bd477ae>:0
at CKAN.Main.ClearChangeSet () [0x00061] in <c787e8ddab794c269a4e41cd1bd477ae>:0
at CKAN.Main.CancelChangesButton_Click (System.Object sender, System.EventArgs e) [0x00001] in <c787e8ddab794c269a4e41cd1bd477ae>:0
at System.Windows.Forms.Control.OnClick (System.EventArgs e) [0x00019] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.Button.OnClick (System.EventArgs e) [0x0001e] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.ButtonBase.OnMouseUp (System.Windows.Forms.MouseEventArgs mevent) [0x00069] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.Button.OnMouseUp (System.Windows.Forms.MouseEventArgs mevent) [0x00000] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.Control.WmLButtonUp (System.Windows.Forms.Message& m) [0x00078] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.Control.WndProc (System.Windows.Forms.Message& m) [0x001b4] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.ButtonBase.WndProc (System.Windows.Forms.Message& m) [0x00037] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.Button.WndProc (System.Windows.Forms.Message& m) [0x00000] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.Control+ControlWindowTarget.OnMessage (System.Windows.Forms.Message& m) [0x00000] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.Control+ControlNativeWindow.WndProc (System.Windows.Forms.Message& m) [0x0000b] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0
at System.Windows.Forms.NativeWindow.WndProc (System.IntPtr hWnd, System.Windows.Forms.Msg msg, System.IntPtr wParam, System.IntPtr lParam) [0x00085] in <8aef12e05d8341c2ba53ae50a7bb2ab8>:0

GUI/GUIMod.cs

HebaruSan · 2019-05-06T21:31:15Z

Can you be more specific about the steps to crash? If I cancel out of uninstalling 6 Crew Science Lab, it just goes back to the mod list for me, no crash.

DasSkelett · 2019-05-06T21:41:13Z

Be on Kubuntu (may or may not be relevant), have 6CSL installed
Unselect 6CSL in the modlist (so it deinstalls)
Click Apply
Changeset tab opens. You see two mods, MM and 6CSL.
Click clear in the lower right corner.
CKAN hard-crashes with the given error.

Let me try to find the exact line with some debugging.

HebaruSan · 2019-05-06T21:44:25Z

The reason for the auto removal action should be fixed now.

HebaruSan · 2019-05-06T21:45:22Z

As for the crash, maybe Mono doesn't support indexing a row's .Cells collection by name? There are other options...

DasSkelett · 2019-05-06T21:55:45Z

Selecting Add available updates works, and if I'm not mistaken, that uses reference by column name too?

HebaruSan · 2019-05-06T21:59:28Z

That calls into the exact same function that's crashing, so if it's behaving differently, then I have no idea.

CKAN/GUI/Main.cs

Lines 569 to 574 in df8fb70

    
               var mod = (GUIMod)row.Tag; 
        
               if (mod.HasUpdate) 
        
               { 
        
                   MarkModForUpdate(mod.Identifier); 
        
               } 
        
           }

CKAN/GUI/MainModList.cs

Lines 318 to 328 in df8fb70

    
           public void MarkModForUpdate(string identifier) 
        
           { 
        
               Util.Invoke(this, () => _MarkModForUpdate(identifier)); 
        
           } 
        
           public void _MarkModForUpdate(string identifier) 
        
           { 
        
               DataGridViewRow row = mainModList.full_list_of_mod_rows[identifier]; 
        
               var mod = (GUIMod)row.Tag; 
        
               mod.SetUpgradeChecked(row, true); 
        
           }

DasSkelett · 2019-05-06T22:07:18Z

One more thing: If you downgrade a mod, the auto-installed tag is lost.
Is it intended this way? I'm honestly not sure how it should be handled.
On one side, the user definitely does "manual", non-default changes to a mod, so he might want to manage it himself.
On the other side, it's just a simple version change, like updating a mod, that doesn't per se mean "Want to keep that mod even if the dependents are removed", and he could always unset the checkbox himself.

HebaruSan · 2019-05-06T22:11:47Z

I didn't consciously consider downgrading, but it doesn't sound broken to me. When in doubt, there's a column where the user can change it.

DasSkelett · 2019-05-06T22:25:42Z

Setting the "UpdateCol" back to 2 fixes it, yes. Confusing that it doesn't crash when coming via MarkModForUpdate()

HebaruSan · 2019-05-06T22:26:04Z

Well, here's the mono code that's throwing that exception:

				if (columnName == null)
					throw new ArgumentNullException ("columnName");


				foreach (DataGridViewCell cell in base.List) {
					if (string.Compare (cell.OwningColumn.Name, columnName, true) == 0)
						return cell;
				}


				throw new ArgumentException (string.Format (
					"Column name {0} cannot be found.",
					columnName), "columnName");

https://github.com/mono/mono/blob/a41be288719e56df153158faba0ad3c27af2e896/mcs/class/System.Windows.Forms/System.Windows.Forms/DataGridViewCellCollection.cs#L67-L77

Looks like an NRE would mean that either cell or cell.OwningColumn is null. I'm going to switch it to a safer version of numeric indices, since that just uses a regular array lookup.

https://github.com/mono/mono/blob/a41be288719e56df153158faba0ad3c27af2e896/mcs/class/System.Windows.Forms/System.Windows.Forms/DataGridViewCellCollection.cs#L45-L58

DasSkelett · 2019-05-06T22:36:15Z

Yep works!
Last thing, probably for a future PR:
Would be nice to be able to change the auto-installed flag at least in the command line UI too, and also consoleUI wouldn't be that bad I think.

HebaruSan · 2019-05-06T22:49:18Z

Yeah, that's needed of course. I'm not sure how it should look, though. I guess another status symbol representing auto installed? And then a command / keystroke to toggle.

DasSkelett · 2019-05-06T22:55:36Z

The toggle option / keystroke could be "hidden" inside the details view, to avoid cluttering the "main" modlist view.
For the symbol, well, a simple 'a'? Or an '@' if it should look a bit fancy?

HebaruSan · 2019-05-06T22:56:36Z

For the symbol, well, a simple 'a'? Or an '@' if it should look a bit fancy?

I'm thinking it might be nice to adopt the same symbol in both CmdLine and ConsoleUI, and CmdLine already uses "A" for manually installed mods. So "a" might be a bit confusing.

HebaruSan · 2019-05-06T22:59:31Z

The other wrinkle is that unlike the existing status symbols in both UIs, this one would need to work alongside the existing ones, since an auto installed mod can also be upgradeable, broken, etc. So e.g. if we used "@", then CmdLine would show an upgradeable auto installed mod as "^@", a normal auto installed mod as "-@", etc.

EDIT: Or, maybe treating it as an alternate form of "-" is sufficient. If a mod is upgradeable, do I really care that it's also auto installed?

HebaruSan · 2019-05-07T01:01:27Z

@DasSkelett, I'm going to add the CmdLine and ConsoleUI changes here. Would you mind un-approving so we can track the review of those changes?

DasSkelett · 2019-05-07T05:49:13Z

Lemme figure out how to do that....

DasSkelett

Can't remove my previous approval, but I can approve the new commits!
Work fine.

politas · 2019-05-08T04:23:20Z

Oooh, excellent! I've been meaning to raise an issue for this.

HebaruSan added Enhancement New features or functionality GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Pull request Registry Issues affecting the registry Relationships Issues affecting depends, recommends, etc. labels May 6, 2019

Olympic1 removed the Enhancement New features or functionality label May 6, 2019

HebaruSan added the Enhancement New features or functionality label May 6, 2019

HebaruSan force-pushed the feature/auto-remove-auto-inst branch from 7df5118 to 398d01b Compare May 6, 2019 04:31

DasSkelett reviewed May 6, 2019

View reviewed changes

GUI/GUIMod.cs Outdated Show resolved Hide resolved

DasSkelett approved these changes May 6, 2019

View reviewed changes

DasSkelett requested review from DasSkelett and removed request for DasSkelett May 7, 2019 05:44

DasSkelett approved these changes May 7, 2019

View reviewed changes

Auto-uninstall auto-installed modules

f2f40cb

HebaruSan force-pushed the feature/auto-remove-auto-inst branch from f4c1c4a to f2f40cb Compare May 7, 2019 15:39

HebaruSan merged commit f2f40cb into KSP-CKAN:master May 7, 2019

HebaruSan added a commit that referenced this pull request May 7, 2019

Merge #2753 Auto-uninstall auto-installed modules

f2209e2

HebaruSan deleted the feature/auto-remove-auto-inst branch May 7, 2019 15:51

Olympic1 removed Enhancement New features or functionality Pull request labels May 7, 2019

HebaruSan mentioned this pull request May 8, 2019

Uninstalling a mod should also prompt user about uninstalling its unused dependencies #978

Closed

This was referenced May 8, 2019

Feature request: Show reverse depends relationships KSP-CKAN/NetKAN#5646

Closed

Hide auto-installed checkbox for auto-detected mods #2758

Merged

HebaruSan mentioned this pull request May 20, 2019

Don't assume string params to Install are identifiers #2764

Merged

HebaruSan mentioned this pull request Jun 22, 2019

Mark auto-install correctly when installing from .ckan file #2793

Merged

HebaruSan mentioned this pull request Feb 17, 2020

Treat metapackage depends as not auto installed #3008

Merged

HebaruSan mentioned this pull request May 16, 2020

Inconsistency Kraken when installing OuterPlanets #930

Open

This was referenced Aug 21, 2022

Changeset page: Repurpose the Description Column #1268

Closed

Fix auto removal when installing w/ deps #3643

Merged

This was referenced Nov 16, 2022

[Bug] GUI freezes on switch to changeset tab after refresh removing ZeroMiniAVC dependencies #3707

Closed

Fix GUI freeze with non-empty changeset at startup #3708

Merged

HebaruSan mentioned this pull request Nov 25, 2022

Use changeset tab for reinstall #3726

Merged

HebaruSan mentioned this pull request Apr 17, 2023

Confusion over meaning of auto-installed checkbox column #3826

Closed

1 task

HebaruSan mentioned this pull request May 20, 2023

Tooltip for auto-installed checkboxes #3842

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto-uninstall auto-installed modules #2753

Auto-uninstall auto-installed modules #2753

HebaruSan commented May 6, 2019 •

edited

Loading

DasSkelett left a comment

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019 •

edited

Loading

HebaruSan commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

HebaruSan commented May 6, 2019 •

edited

Loading

HebaruSan commented May 7, 2019

DasSkelett commented May 7, 2019

DasSkelett left a comment

politas commented May 8, 2019

Auto-uninstall auto-installed modules #2753

Auto-uninstall auto-installed modules #2753

Conversation

HebaruSan commented May 6, 2019 • edited Loading

Motivation

Changes

GUI

CmdLine

ConsoleUI

DasSkelett left a comment

Choose a reason for hiding this comment

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019 • edited Loading

HebaruSan commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

DasSkelett commented May 6, 2019

HebaruSan commented May 6, 2019

HebaruSan commented May 6, 2019 • edited Loading

HebaruSan commented May 7, 2019

DasSkelett commented May 7, 2019

DasSkelett left a comment

Choose a reason for hiding this comment

politas commented May 8, 2019

HebaruSan commented May 6, 2019 •

edited

Loading

DasSkelett commented May 6, 2019 •

edited

Loading

HebaruSan commented May 6, 2019 •

edited

Loading